Fixing a few issues #8

AcidRaZor · 2023-12-22T03:00:55Z

Updated ReadMe.md with information to get up and running
Updated Dockerfile for initial docker-compose up command to work successfully
Updated Name including Title to keep it consistent when renaming a directory
Changed PROPFIND to an IActionResult to return 404 NotFound on some items (Windows related)
Changed PathService to avoid unnecessary overhead on .First() and .Last() LINQ commands
Added a more reliable check if a directory is an actual directory
Added a more reliable check for invalid characters in a directory (resource name) on creation
Fixed an issue where, if a directory didn't exist, an exception was thrown (PathService)
Fixed an issue where, if a directory were empty or not found, an exception was thrown (WebDavService)
Fixed an issue where VirtualStorageService was referencing the parent directory Id when getting directory information
Fixes an issue on directory rename where it referenced the parent directory instead of the one being renamed

- Updated to .NET 7 images as project changed to use .NET 7 (from .NET 6) - Fixed COPY where it was copying WebDavService.Application CSPROJ that doesn't exist (or was renamed previously)

…tead to get rid of some unnecessary overhead (nitpicky I know) - Made the isDirectory check more reliable, found that in a WebDAV environment (Windows PC mapped drive to server), it picked up requests as not being a directory, causing some issues as the actual request (path) didn't end with "/" from GetPath() on the entry-point - Added an if-check to check if the item is, in fact, a directory before removing it from the list of directories. This made navigation possible to the last most child directory, otherwise it would show nothing

…s example

…first to see if the directory exists, if it didn't gave an error because the database is expected to always return a value

…empty. This happens when the directory or file doesn't exist. As part of the sequence (maybe specific to Windows mapped drives), it does a PROPFIND before running MKCOL/Get etc.

…e WebDAV response when an item doesn't exist. PROPFIND runs before MKCOL so it's important not to throw an exception in this sequence

… requesting dumb windows files (Usually through Mapped Drive on Windows machines)

…hen Windows, by default, creates "New Folder" for you to rename when creatng. This should be cross-platform compatible.

…directory info instead of the actual directory referenced

…ent instead of the directory being renamed - Updated Name to match Title

AcidRaZor · 2023-12-22T08:01:31Z

Forgot to update the tests

JingoC · 2023-12-22T06:28:52Z

WebDavServer.Infrastructure.FileStorage/Services/FileStorageService.cs

        }
+        private bool HasInvalidChars(string directoryName)
+        {
+            return (!string.IsNullOrEmpty(directoryName) && directoryName.IndexOfAny(Path.GetInvalidPathChars()) >= 0);


remove extra parentheses "(" ")"

JingoC · 2023-12-22T06:36:50Z

WebDavServer.Infrastructure.FileStorage/Services/FileStorageService.cs


            return "text/plain";
        }
+        private bool HasInvalidChars(string directoryName)


private static

Any reason why it should be static? Im trying to learn clean architecture as I go

This is not about clean architecture, but about clean code. Since the method does not use class members, it should be defined as static

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/static#example---static-field-and-method

JingoC · 2023-12-22T06:52:17Z

WebDavServer.Infrastructure.FileStorage/Services/PathService.cs

            var directories = relativePathTrim.Split("/").Where(x => !string.IsNullOrEmpty(x)).ToList();

-            var isDirectory = relativePathTrim.EndsWith("/");
+            var isDirectory = relativePathTrim == "/" || !Path.HasExtension(relativePathTrim);


The file name does not necessarily contain extensions (for example, "my_file_name"), you need to check for the presence of a "/" at the end.

All path names, even directories via WebDAV comss through with no "/" at the end of the name, which meant that all directories other than root were seen as files, and took the wrong code-path

Thats problem concrete client, by specification required / for collections (folder)

http://www.webdav.org/specs/rfc4918.html#rfc.section.5.2.p.8

Not from what I observed. URL does end with "/" (even files), but path via GetPath doesnt. Might be the way Microsoft implemented WebDAV via the map network drive functionality, which would be strange not to follow RFC (but not unlike Microsoft). Other WebDAV server we are running follows RFC and works with that though. Maybe I am missing something.

JingoC · 2023-12-24T07:24:47Z

WebDavServer.WebApi/Controllers/WebDavController.cs

-        public async Task<string> PropfindAsync(string? path, CancellationToken cancellationToken)
+        public async Task<IActionResult> PropfindAsync(string? path, CancellationToken cancellationToken)
        {
+            if (path is not null && (path.Contains("desktop.ini") ||


Why do these files need to be hidden?

Annoying windows explorer-only stuff that gets checked on a file system that would never contain them *shrug :)

Place the list of files in a separate class, something like this

public static class WebDavConsts { public static string[] ExcludeHiddenFiles = new[] { ... } } ... // use if (WebDavConsts.ExcludeHiddenFiles.Any(path.Contains)) { ... }

JingoC · 2023-12-24T07:45:58Z

WebDavServer.Infrastructure.FileStorage/Services/PathService.cs

            if (item == null)
            {
-                throw new FileStorageException(ErrorCodes.PartOfPathNotExists);
+                return default;


use http://www.webdav.org/neon/litmus/ for testing all changes, especially this place

JingoC · 2023-12-24T10:00:06Z

WebDavServer.Infrastructure.FileStorage/Services/VirtualStorageService.cs

                .Where(x => x.IsDirectory)
                .Where(x => x.Title == srcPath.ResourceName)
-                .Where(x => x.DirectoryId == directoryId)
+                .Where(x => x.Id == directoryId)


in this filter, the item is searched by the parent directory in which it should be located and the name of the items, and not by its id. If we had an id here, it would be enough to do Find(id)

From my observations, in sub-directories, that is not in root, this results in not finding any information from the database. Works fine if all directories will always be in root

AcidRaZor and others added 13 commits December 22, 2023 13:46

Update Dockerfile:

9741952

- Updated to .NET 7 images as project changed to use .NET 7 (from .NET 6) - Fixed COPY where it was copying WebDavService.Application CSPROJ that doesn't exist (or was renamed previously)

Updated Readme with steps to get this up and running with the Postgre…

c33961e

…s example

- Fixed an issue where, when running MKCOL, a PROPFIND check is done …

5fbc307

…first to see if the directory exists, if it didn't gave an error because the database is expected to always return a value

- Fixes an issue where an exception is thrown when propertiesList is …

5679637

…empty. This happens when the directory or file doesn't exist. As part of the sequence (maybe specific to Windows mapped drives), it does a PROPFIND before running MKCOL/Get etc.

- Fixed an issue where an exception was thrown that's unhandled in th…

ee8bc93

…e WebDAV response when an item doesn't exist. PROPFIND runs before MKCOL so it's important not to throw an exception in this sequence

- Changed PROPFIND to be an IActionResult to return 404 NotFound when…

57e174c

… requesting dumb windows files (Usually through Mapped Drive on Windows machines)

- Fixed an issue where the RegEx ignored valid filenames. Important w…

7f92ef2

…hen Windows, by default, creates "New Folder" for you to rename when creatng. This should be cross-platform compatible.

- Fixed an issue where, it's checking the parent Id when getting the …

63b789c

…directory info instead of the actual directory referenced

- Align with previous changes for consistency

a9c554a

Update ReadMe.md

0647bde

Accidentially inverted the check for HasInvalidChars

808e856

- Fixes an issue on directory rename where it was referencing the par…

21fc159

…ent instead of the directory being renamed - Updated Name to match Title

JingoC requested changes Dec 24, 2023

View reviewed changes

JingoC reviewed Dec 24, 2023

View reviewed changes

Fixing a few issues #8

Are you sure you want to change the base?

Fixing a few issues #8

Uh oh!

Conversation

AcidRaZor commented Dec 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AcidRaZor commented Dec 22, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AcidRaZor commented Dec 22, 2023 •

edited

Loading